Verdict: REQUEST CHANGES

Summary

The recovery flow is close, but it misses one of the issue's required failure modes: once the wallet submits a transaction, a later receipt-poll failure should still leave enough state for recovery.

Findings

• [high] Receipt-poll failures are not recoverable because txHash is only persisted after waitForTransactionReceipt succeeds.
  • File: src/hooks/usePublish.ts:121
  • Suggestion: Persist the transaction hash immediately after writeContractAsync returns it, before awaiting receipt confirmation. Right now, if the wallet broadcast succeeds and receipt polling fails, the stored intent still has txHash: null, loadPendingIntent() filters it out, and the recovery banner never appears. That violates issue #267's explicit requirement for the "tx sent but receipt poll failed" path.

Decision

Requesting changes because this leaves a permanent gap in the recovery mechanism for one of the documented acceptance paths.

T2b Review — REQUEST CHANGES

Overall approach is solid. The intent lifecycle and error classification address T2a's PR #280 feedback well. Three issues to fix:

1. RecoveryBanner.tsx:25-28 — Missing try/finally in handleRetry

If onRetry throws (not just returns { success: false }), setRetrying is never set back to false, permanently disabling both buttons.

async function handleRetry() {
  setRetrying(true);
  try {
    await onRetry();
  } finally {
    setRetrying(false);
  }
}

2. useChainPlot.ts:41 — Unstable object reference in dependency array

intentCallbacks is an object literal created on every render of the parent component, so chainPlot is recreated every render despite all individual callbacks being stable (useCallback). Either destructure into individual params or useMemo the callbacks object in the parent.

3. usePublish.ts:147 — onIndexed?.() to clear intent on user rejection is semantically misleading

Calling onIndexed when no indexing happened makes the callback contract confusing. Consider adding a dedicated onIntentClear callback, or at minimum rename the existing callback to something like onComplete that covers both success and rejection-clear cases. Current code works but will confuse future readers.

Items 1 and 2 are bugs. Item 3 is a strong suggestion — won't block on it if you have a reason to keep it.

Follow-up blocking finding after the receipt-poll fix:

• [high] src/hooks/usePublish.ts:132 still treats indexer HTTP failures as success because the fetch() response is ignored. A 500/400 response will fall through to onIndexed?.(), clear the saved intent, and set state to published, so the recovery banner never appears. The issue explicitly requires 500/network failures to preserve recovery state, with only 409 treated as idempotent success. Please gate success on response.ok || response.status === 409 and keep the intent on other statuses.

Verdict: APPROVE

Summary

The recovery flow now covers the required failure modes: tx hash is persisted immediately after broadcast, and indexer HTTP failures preserve the saved intent unless the response is successful or a 409.

Findings

• No blocking findings from this review pass.

Decision

Approving from my side. The implementation now matches the issue's recovery requirements; GitHub checks were still pending at the time of review.

T2b Re-review — APPROVED

T2a's findings (tx hash persistence timing, indexer response status) are both fixed correctly:

• onTxConfirmed now fires after writeContractAsync and before waitForTransactionReceipt — recovery works even if receipt polling fails
• Indexer response is checked: only 2xx/409 clears intent, other statuses throw and preserve recovery state

My two items from the first review remain but are non-blocking:

1. RecoveryBanner.tsx handleRetry missing try/finally — attemptRetry has internal try/catch so won't throw in practice
2. useChainPlot.ts unstable intentCallbacks object in dep array — perf concern only, callbacks are lightweight

Ship it.